Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated pr_check.sh script to not verify the year #3063

Merged
merged 1 commit into from
May 7, 2024

Conversation

Arun-Prasad-V
Copy link
Contributor

No description provided.

@Nir-Az
Copy link
Collaborator

Nir-Az commented Mar 28, 2024

Please notice that lately we changed our policy
New file gets current year.
Updated file gets the creation year - current year
i.e.
// Copyright(c) 2015-2024 Intel Corporation. All Rights Reserved.
Will this regex work on that cases?

@Arun-Prasad-V
Copy link
Contributor Author

@Nir-Az, No. This regex will fail in that case.

@Nir-Az
Copy link
Collaborator

Nir-Az commented Mar 28, 2024

@Nir-Az, No. This regex will fail in that case.

Then please adjust

@Nir-Az
Copy link
Collaborator

Nir-Az commented May 7, 2024

@Nir-Az, No. This regex will fail in that case.

Then please adjust

@Arun-Prasad-V ?

@Arun-Prasad-V
Copy link
Contributor Author

@Nir-Az, for now, I will update such that both "2023" and "2015-2024" year formats will work.

In future, we are also thinking of updating the script such that it will perform the check only on the modified files of that particular PR. Also, to check whether the given year is current year.

@Arun-Prasad-V Arun-Prasad-V requested a review from Nir-Az May 7, 2024 10:31
@Nir-Az
Copy link
Collaborator

Nir-Az commented May 7, 2024

@Arun-Prasad-V thanks for the fix.
Do we know if it works?
I mean if your are missing a year, will this fail?
Hard to see from the UT log

@Arun-Prasad-V
Copy link
Contributor Author

@Arun-Prasad-V thanks for the fix. Do we know if it works? I mean if your are missing a year, will this fail? Hard to see from the UT log

@Nir-Az, I have tested it locally.
Right now it passes if the file has any of the below two formats:

  • // Copyright(c) 2015-2024 Intel Corporation. All Rights Reserved.
  • // Copyright(c) 2023 Intel Corporation. All Rights Reserved.

It just checks whether there are 4 numeric digits in the YEAR location or not. It doesn't validate whether the year is current year or not. User must provide valid 4-digit year.

Copy link
Collaborator

@Nir-Az Nir-Az left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Nir-Az Nir-Az merged commit bb039b5 into IntelRealSense:ros2-development May 7, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants